Skip to content

Use a covariant type for detectors argument#5135

Merged
xrmx merged 2 commits intoopen-telemetry:mainfrom
nickstenning:fix-get-aggregated-resource-typing
Apr 30, 2026
Merged

Use a covariant type for detectors argument#5135
xrmx merged 2 commits intoopen-telemetry:mainfrom
nickstenning:fix-get-aggregated-resource-typing

Conversation

@nickstenning
Copy link
Copy Markdown
Contributor

@nickstenning nickstenning commented Apr 23, 2026

Description

This updates the type annotation for get_aggregated_resources's detectors argument to use typing.Sequence, which is covariant in its type parameter and thus has the expected behaviour for an input type.

typing.List is invariant in its type parameter, which means that a simple list of ResourceDetector subclasses is not a list[ResourceDetector].

Some typecheckers are more lenient than others, but ty enforces this and thus the following code does not typecheck despite being semantically correct.

from opentelemetry.sdk.resources import (
    OsResourceDetector,
    ProcessResourceDetector,
    Resource,
    get_aggregated_resources,
)
from opentelemetry.sdk.trace import TracerProvider

resource = Resource.create()

resource_detectors = [
    ProcessResourceDetector(),
    OsResourceDetector(),
]

tracer_provider = TracerProvider(
    resource=get_aggregated_resources(
        detectors=resource_detectors,
        initial_resource=resource,
    ),
)

The full output of ty is as follows:

$ uv run ty check
error[invalid-argument-type]: Argument to function `get_aggregated_resources` is incorrect
  --> test.py:18:9
   |
18 |         detectors=resource_detectors,
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected `list[ResourceDetector]`, found `list[ProcessResourceDetector | OsResourceDetector]`
   |
info: Function defined here
   --> .venv/lib/python3.13/site-packages/opentelemetry/sdk/resources/__init__.py:507:5
    |
507 | def get_aggregated_resources(
    |     ^^^^^^^^^^^^^^^^^^^^^^^^
508 |     detectors: typing.List["ResourceDetector"],
    |     ------------------------------------------ Parameter declared here
    |
info: `list` is invariant in its type parameter
info: Consider using the covariant supertype `collections.abc.Sequence`
info: For more information, see https://docs.astral.sh/ty/reference/typing-faq/#invariant-generics

Found 1 diagnostic

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Run ty check on the example given in the description with the updated opentelemetry-sdk installed.

Does This PR Require a Contrib Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@herin049
Copy link
Copy Markdown
Contributor

Can you please add a Changelog entry.

@github-project-automation github-project-automation Bot moved this to Approved PRs in Python PR digest Apr 23, 2026
Comment thread opentelemetry-sdk/src/opentelemetry/sdk/resources/__init__.py Outdated
This updates the type annotation for `get_aggregated_resources`'s
`detectors` argument to use `collections.abc.Sequence`, which is
covariant in its type parameter and thus has the expected behaviour for
an input type.

`typing.List` is invariant in its type parameter, which means that a
simple list of `ResourceDetector` subclasses is not a
`list[ResourceDetector]`.

Some typecheckers are more lenient than others, but [ty] enforces this
and thus the following code does not typecheck despite being
semantically correct.

    from opentelemetry.sdk.resources import (
        OsResourceDetector,
        ProcessResourceDetector,
        Resource,
        get_aggregated_resources,
    )
    from opentelemetry.sdk.trace import TracerProvider

    resource = Resource.create()

    resource_detectors = [
        ProcessResourceDetector(),
        OsResourceDetector(),
    ]

    tracer_provider = TracerProvider(
        resource=get_aggregated_resources(
            detectors=resource_detectors,
            initial_resource=resource,
        ),
    )

The full output of ty is as follows:

    $ uv run ty check
    error[invalid-argument-type]: Argument to function `get_aggregated_resources` is incorrect
      --> test.py:18:9
       |
    18 |         detectors=resource_detectors,
       |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Expected `list[ResourceDetector]`, found `list[ProcessResourceDetector | OsResourceDetector]`
       |
    info: Function defined here
       --> .venv/lib/python3.13/site-packages/opentelemetry/sdk/resources/__init__.py:507:5
        |
    507 | def get_aggregated_resources(
        |     ^^^^^^^^^^^^^^^^^^^^^^^^
    508 |     detectors: typing.List["ResourceDetector"],
        |     ------------------------------------------ Parameter declared here
        |
    info: `list` is invariant in its type parameter
    info: Consider using the covariant supertype `collections.abc.Sequence`
    info: For more information, see https://docs.astral.sh/ty/reference/typing-faq/#invariant-generics

    Found 1 diagnostic

[ty]: https://docs.astral.sh/ty/reference/typing-faq/#invariant-generics
@nickstenning nickstenning force-pushed the fix-get-aggregated-resource-typing branch from 883ea90 to 262f275 Compare April 23, 2026 19:59
@nickstenning
Copy link
Copy Markdown
Contributor Author

This should be good to go. Let me know if I need to do anything else :)

@xrmx xrmx moved this from Approved PRs to Ready for merge in Python PR digest Apr 30, 2026
@xrmx xrmx merged commit 0844b5e into open-telemetry:main Apr 30, 2026
466 checks passed
@github-project-automation github-project-automation Bot moved this from Ready for merge to Done in Python PR digest Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants